-
Notifications
You must be signed in to change notification settings - Fork 1
feat 97 메모 수정기능 추가 #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "feat/#97/\uBA54\uBAA8-\uC218\uC815\uAE30\uB2A5-\uCD94\uAC00"
Conversation
seola12e
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
북마크 관련 api들 모두 다시 테스트 부탁드려요 수고하셨습니다
PR 제목도 수정 부탁드려요
| updated = true; | ||
| } | ||
|
|
||
| if (request.file() != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated = true;로 값이 수정이 안되는 부분도 있네요 확인부탁드려요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한 해당 메소드 마지막에 있는 updated 값이 false일 때 커스텀 예외처리 부탁드립니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정했습니다
choes0101
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
jj0526
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다 수정 부탁드려요
| BOOKMARK_TAG_MINIMUM_REQUIRED_EXCEPTION(400, "최소 1개 이상의 태그를 선택해야 합니다."), | ||
| BOOKMARK_TAG_COUNT_EXCEEDED_EXCEPTION(400, "태그는 최대 3개까지만 선택할 수 있습니다."); | ||
| BOOKMARK_TAG_COUNT_EXCEEDED_EXCEPTION(400, "태그는 최대 3개까지만 선택할 수 있습니다."), | ||
| Bookmark_Update_Field_Empty_Exception(400, "업데이트할 필드가 존재하지 않습니다."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum 이름은 모든 글자를 대문자로 부탁드려요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정하겠습니다
| NotificationSaveRequest saveRequest = new NotificationSaveRequest( | ||
| request.notification().notifyAt() | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매퍼 사용 부탁드립니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정이 제대로 안되었는데 확인 부탁드립니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다시 수정했습니다
| if (request.url() != null && !request.url().trim().isEmpty()) { | ||
| bookmark.updateUrl(request.url()); | ||
| updated = true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thumbnailUrl, platform, faviconUrl, url은 모두 url이 바뀌었기에 일어나기에 검증 처리를 통해 한번에 이루어져야 한다고 생각됩니다. 하지만 시간상 이후에 리팩토링으로 부탁드립니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵
jj0526
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~
✨ 관련 이슈
🔎 작업 내용
📷 이미지 첨부
✅ Check List